Support RPMs installing in `/opt` and `/usr/local` #4728

jlebon · 2023-12-14T21:51:07Z

This solves the /opt problem by using the new state overlay concept in
OSTree: an overlay filesystem is mounted on top of /usr/lib/opt and
the upper dir is automatically "rebased" whenever new content comes in.
Concretely, this means that app state is carried forward, all while
allowing the (OSTree-managed) package contents to be updated.

We also solve the /usr/local problem the same way. The app state issue
isn't really present there, but /usr/local has traditionally been
system state. We want to keep supporting dropping files there all while
also supporting shipping OSTree-owned content.

See also: ostreedev/ostree#3113
Fixes: #233

openshift-ci · 2023-12-14T21:51:11Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

jlebon · 2023-12-14T21:51:26Z

Requires: ostreedev/ostree#3120

This also still needs tests.

jlebon · 2023-12-14T21:55:20Z

Fixes: #233

I know this is quite optimistic. :)

But I think this is pretty close to a fix for it, even if there are some subtle caveats. With the experimental knob there, I'm hoping we can get feedback on how well this works in practice.

cgwalters

Just starting to dig into this...seems like we could split out a bit of this as prep commits?

Makefile-rpm-ostree.am

cgwalters · 2024-01-02T20:36:06Z

rust/src/importer.rs

-    if path.starts_with("/usr/") && !path.starts_with("/usr/local") {
+    if path.starts_with("/usr/") {


Seems like this change needs to be gated on us detecting that a state overlay is enabled?

If we want to be strict, yes. I didn't bother with this because /usr/local RPMs are known broken right now anyway and this PR is working to improve the situation. With this hunk but without state overlays enabled, it'll still be broken, but in a different way (though true, in a more subtle way than the current explicit error message users get -- content in /usr/local will just be deleted in postprocessing when replaced by the /var/usrlocal symlink).

Can pipe through the needed flag to retain current behaviour if wanted.

tests/compose/test-state-overlays.sh

jlebon · 2024-01-24T18:25:04Z

Now rebased and with tests!

I'll note that there should be no functional change here if state overlays are not enabled.

jmarrero · 2024-01-24T20:03:10Z

[2024-01-24T19:01:53.335Z] --- FAIL: ext.rpm-ostree.destructive.state-overlays (105.74s)

rust/src/composepost.rs

cgwalters · 2024-01-24T22:57:53Z

rust/src/composepost.rs

+    ];
+
+    UNITS.par_iter().try_for_each(|&unit| {
+        let target = Path::new("..").join(unit);


Huh, doesn't cap-std barf on this? I did this PR a while ago which we use in other places; though maybe it doesn't actually check for ...

Yup, seems to work here! :)

cgwalters · 2024-01-24T22:58:54Z

rust/src/composepost.rs

@@ -955,6 +1008,17 @@ fn convert_path_to_tmpfiles_d_recurse(
    Ok(())
 }

+fn state_overlay_enabled(rootfs_dfd: &cap_std::fs::Dir, state_overlay: &str) -> Result<bool> {


This function in theory could live in ostree's rust code or ostree-ext, not that it matters much in practice. (Please don't move it just for this, but just a thought)

cgwalters · 2024-01-24T23:01:30Z

src/libpriv/rpmostree-core.cxx

@@ -4062,6 +4062,55 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G
  if (!build_rootfs_usrlinks (self, error))
    return FALSE;

+  /* This is purely for making it easier for people to test out the
+   * state-overlay stuff until it's stabilized and part of base composes. */
+  if (g_getenv ("RPMOSTREE_EXPERIMENTAL_FORCE_OPT_USRLOCAL_OVERLAY"))


Wouldn't it be better to document how to do this in a derived container build? The ergonomics around providing an environment variable in this context seems...difficult.

cgwalters · 2024-01-24T23:04:06Z

tests/kolainst/destructive/state-overlays

+    systemctl stop rpm-ostreed
+    unshare -m /bin/bash -c 'mount -o rw,remount /sysroot && sed -i -e "s/refspec=.*/refspec=kolatest/" /ostree/deploy/*/deploy/*.origin'
+
+    # XXX: until ostree v2024.1 hits FCOS


looks like that should happen in a day or two at most

Looks like it hasn't synced to the mirrors yet. I'll keep it in for now and pull it out once it makes it in (whether here or in a follow-up).

cgwalters · 2024-01-24T23:05:53Z

tests/kolainst/destructive/state-overlays

+    mkdir -p /etc/systemd/system/rpm-ostreed.service.d/
+    cat > /etc/systemd/system/rpm-ostreed.service.d/state-overlay.conf <<EOF
+[Service]
+Environment=RPMOSTREE_EXPERIMENTAL_FORCE_OPT_USRLOCAL_OVERLAY=1


Ohhh I see it was for CI testing...ah. WDYT about changing this test to use a container derivation instead? Which could be done in-kola, although...

Or alternatively...this may argue for changing our CI here to have a separate flow which does separate treecompose runs and separate CI off each of them.

Doing this from a container requires ostreedev/ostree-rs-ext#573, no? Or do you mean fixing the directories and enabling the services in a container build but still installing the RPM client-side? That feels like the worst of both worlds. :)

Not too concerned about ergonomics for this. A service dropin isn't too bad a way to opt into experimental stuff, no? Though definitely once we have support in the container path, we should add a test for that.

Re. doing a separate treecompose run, yeah I think we do want that too (and has definitely come up before). Hmm, how about for now instead of adding another run to CI, we exercise it in the fcos-e2e tests since that one also does a full compose and kola tests? We can adapt the test to skip the experimental knob injection if it detects that it's already on in the base compose.

Obviously, this alone is not enough to expose that content but it's a start. Currently as is, it'll get nuked when we replace `/usr/local` by a symlink in postprocessing. A future patch will address that part. Part of coreos#233.

This solves the `/opt` problem by using the new state overlay concept in OSTree: an overlay filesystem is mounted on top of `/usr/lib/opt` and the upper dir is automatically "rebased" whenever new content comes in. Concretely, this means that app state is carried forward, all while allowing the (OSTree-managed) package contents to be updated. We also solve the `/usr/local` problem the same way. The app state issue isn't really present there, but `/usr/local` has traditionally been system state. We want to keep supporting dropping files there all while also supporting shipping OSTree-owned content. See also: ostreedev/ostree#3113 Fixes: coreos#233

HuijingHei

LGTM

cgwalters · 2024-01-25T01:10:39Z

rust/src/composepost.rs

+    let mut db = cap_std::fs::DirBuilder::new();
+    db.recursive(true);
+    db.mode(0o755);
+    let localfs_requires = Path::new("usr/lib/systemd/system/local-fs.target.requires");


This one could probably live as a const given it's referenced twice.

Follow-up from coreos#4728.

Follow-up to coreos#4728.

Since both Prow and CoreOS CI do a compose and run kola tests, we can change one of them to cover some different options. Follow-up to coreos#4728.